Skip to content

Conversation

fregataa
Copy link
Member

@fregataa fregataa commented Sep 5, 2025

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • Mention to the original issue
  • Installer updates including:
    • Fixtures for db schema changes
    • New mandatory config options
  • Update of end-to-end CLI integration tests in ai.backend.test
  • API server-client counterparts (e.g., manager API -> client SDK)
  • Test case(s) to:
    • Demonstrate the difference of before/after
    • Demonstrate the flow of abstract/conceptual models with a concrete implementation
  • Documentation
    • Contents in the docs directory
    • docstrings in public interfaces and type annotations

@fregataa fregataa added this to the 25Q2 milestone Sep 5, 2025
@fregataa fregataa self-assigned this Sep 5, 2025
@fregataa fregataa added the skip:changelog Make the action workflow to skip towncrier check label Sep 5, 2025
@github-actions github-actions bot added size:L 100~500 LoC comp:manager Related to Manager component labels Sep 5, 2025
@fregataa fregataa force-pushed the feat/add-rbac-db-source branch from 4f51346 to 0f600c4 Compare September 5, 2025 09:41
@fregataa fregataa force-pushed the feat/add-processors-based-on-action-types branch from bfdd80a to 2cf1a22 Compare September 5, 2025 09:45
@fregataa fregataa force-pushed the feat/add-rbac-db-source branch 2 times, most recently from 3c9040b to 8ec6b57 Compare September 5, 2025 10:19
from ...models.utils import ExtendedAsyncSAEngine


class PermissionControllerDBSource:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem good that the word "controller" is included in the source.

Comment on lines 69 to 71
role_row = await self._get_role(data.id, db_session)
if role_row is None:
raise ObjectNotFound(f"Role with ID {data.id} does not exist.")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better if you returned it non-Optional from the moment you read it in _get_role.

Comment on lines +101 to +119
j = (
sa.join(
RoleRow,
UserRoleRow,
RoleRow.id == UserRoleRow.role_id,
)
.join(
ObjectPermissionRow,
RoleRow.id == ObjectPermissionRow.role_id,
)
.join(
PermissionGroupRow,
RoleRow.id == PermissionGroupRow.role_id,
)
.join(
PermissionRow,
PermissionGroupRow.id == PermissionRow.permission_group_id,
)
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😭

@fregataa fregataa force-pushed the feat/add-processors-based-on-action-types branch from 2cf1a22 to 4db5743 Compare September 16, 2025 05:15
@fregataa fregataa force-pushed the feat/add-rbac-db-source branch from 8ec6b57 to bfe9e1b Compare September 16, 2025 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:manager Related to Manager component size:L 100~500 LoC skip:changelog Make the action workflow to skip towncrier check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants